Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Dynamic Sound #6600

Closed
wants to merge 18 commits into from
Closed

Dynamic Sound #6600

wants to merge 18 commits into from

Conversation

Git-Nivrak
Copy link
Contributor

@Git-Nivrak Git-Nivrak commented Jul 1, 2024 •
edited
Loading

About the pull request

Makes sounds react to your and its source moving around and become louder\quieter and change direction

Splits sounds into sounds the originate from a location and sounds that originate from a source - sounds that originate from a location will stay put at the location while sounds from a source will move around with the source (For example a Queen screech is a source and being slammed into a wall is a location). Due to this some things will probably not sound right because they should be a source while being a location or vice versa and this will have to be tweaked along when\if this is tested.

You still can't hear sounds if you were completely out of range when they started.

Explain why it's good for the game

It's really cool if it doesn't lag the game to hell and back

Testing Photographs and Procedure

Screenshots & Videos

Put screenshots and videos here with an empty line between the screenshots and the <details> tags.

Changelog

🆑
add: Sounds will now change as you move around their source and become quieter \ louder and change direction
code: Added support for sounds having a movable source, simply put the atom as the source
/:cl:

@Git-Nivrak Git-Nivrak requested a review from fira as a code owner July 1, 2024 12:24
@github-actions github-actions bot added Feature Feature coder badge Code Improvement Make the code longer labels Jul 1, 2024
@Git-Nivrak
Copy link
Contributor Author

OD linter should be fixed as soon as it is updated (OpenDreamProject/OpenDream#1870)

@Doubleumc
Copy link
Contributor

I was looking in to a system similar to this. Some observations that may be helpful:

  • When you send a SOUND_UPDATE to a sound channel, it applies (almost) ALL of its variables to the playing sound, such as volume which defaults to 100. It is important that the updating sound have the same settings as the playing sound to maintain volume/pitch/etc consistency. Your current system handles this, just noting that it is important that it keeps doing it.
  • If performance is a concern the vast majority of sounds don't need the extra handling, they're short enough or soft enough that people won't notice any positional offset. For example, the M41 firing sound is less than a second and people probably won't notice it being off by less than a second's worth of movement. There's no good way to test this in code (getting a sound's duration is very inconvenient) so it would be up to the playsound caller to indicate whether it needs positional updates or not.

@nauticall nauticall added the Testmerge Candidate we'll test this while you're asleep and the server has 10 players label Jul 4, 2024
@cm13-github
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@cm13-github cm13-github added the Merge Conflict PR can't be merged because it touched too much code label Jul 6, 2024
@cm13-github
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@cm13-github cm13-github removed the Merge Conflict PR can't be merged because it touched too much code label Jul 6, 2024
cm13-github added a commit that referenced this pull request Jul 10, 2024
@cm13-github
TGS Test Merge (#6600)
05140c1
@private-tristan
Copy link
Contributor

Current TM is borked and everything sounds like it's happening on your tile/adjacent

@Git-Nivrak
fixes
db03735 
fixes every sound being place ontop of you
@Git-Nivrak
reduces smoothing
4293e2e
@Git-Nivrak
Update soundOutput.dm
973ac7a
cm13-github added a commit that referenced this pull request Jul 11, 2024
@cm13-github
TGS Test Merge (#6600)
57a3d89
@private-tristan
Copy link
Contributor

Sound desyncs during lag.

PVP.CM.runner.needshelp.dynamic.sounds.desync.mp4

@cm13-issue-bot cm13-issue-bot mentioned this pull request Jul 11, 2024
Closed
Doubleumc
Doubleumc reviewed Jul 12, 2024
View reviewed changes
code/game/sound.dm
@@ -36,7 +38,7 @@
//status: the regular 4 sound flags
//falloff: max range till sound volume starts dropping as distance increases

/proc/playsound(atom/source, soundin, vol = 100, vary = FALSE, sound_range, vol_cat = VOLUME_SFX, channel = 0, status , falloff = 1, y_s_offset,x_s_offset)
/proc/playsound(atom/source, soundin, vol = 100, vary = FALSE, sound_range, vol_cat = VOLUME_SFX, channel = 0, status , falloff = 1, y_s_offset,x_s_offset, smoothing=TRUE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/proc/playsound(atom/source, soundin, vol = 100, vary = FALSE, sound_range, vol_cat = VOLUME_SFX, channel = 0, status , falloff = 1, y_s_offset,x_s_offset, smoothing=TRUE)
/proc/playsound(atom/source, soundin, vol = 100, vary = FALSE, sound_range, vol_cat = VOLUME_SFX, channel = 0, status , falloff = 1, y_s_offset,x_s_offset, use_smoothing=TRUE)

You named the argument smoothing but are trying to call it with use_smoothing

@Git-Nivrak
Copy link
Contributor Author

The server can't take it
Perhaps if byond had an easy way to get sound file's length but until then gonna close it

@Git-Nivrak Git-Nivrak closed this Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zonespace27 Zonespace27 Zonespace27 left review comments

@Doubleumc Doubleumc Doubleumc left review comments

@fira fira Awaiting requested review from fira fira is a code owner

Assignees
No one assigned
Labels
Code Improvement Make the code longer Feature Feature coder badge Testmerge Candidate we'll test this while you're asleep and the server has 10 players
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Git-Nivrak @Doubleumc @cm13-github @private-tristan @Zonespace27 @nauticall